feat: persistent-zval helpers (deep-copy zval trees across threads)#2366
feat: persistent-zval helpers (deep-copy zval trees across threads)#2366nicolas-grekas wants to merge 1 commit intophp:mainfrom
Conversation
Second step of the split suggested in php#2287: land the persistent-zval subsystem as a standalone, reviewable header, independent of background workers. This is the subsystem most likely to hide latent refcount or memory-lifetime bugs; reviewing it in isolation is higher-signal than finding issues inside a 3k-line diff. ## What - persistent_zval.h (renamed from the bg_worker_vars.h draft, prefix dropped for generality): - persistent_zval_validate: whitelist (scalars, arrays of allowed values, enum instances). Everything else fails fast. - persistent_zval_persist: deep-copy request -> persistent (pemalloc) memory. Fast paths baked in: interned strings shared, opcache- immutable arrays passed by pointer without copying or owning. - persistent_zval_free: deep-free; skips interned strings and immutable arrays (borrowed, not owned). - persistent_zval_to_request: deep-copy persistent -> fresh request memory. Enums re-resolved by class + case name on each read. - frankenphp.c: header included only when FRANKENPHP_TEST_HOOKS is defined. First real consumer (background workers) drops the guard. - Test hook gated on FRANKENPHP_TEST_HOOKS: - PHP function frankenphp_test_persist_roundtrip(mixed): mixed runs validate -> persist -> to_request -> free and returns the result. - Registered via zend_register_functions at MINIT so it never appears in ext_functions[] and never ships in production builds. - CI workflows set -DFRANKENPHP_TEST_HOOKS in CGO_CFLAGS (tests.yaml + sanitizers.yaml). windows.yaml is the release build, not a test runner, and stays untouched. ## Notes - Build verified both without the flag (production path, no unused-function warnings) and with it (test path). - The FRANKENPHP_TEST_HOOKS guard around the header include goes away in the PR that lands the first real caller; the test hook itself goes away in that same step once end-to-end tests cover the code paths.
nicolas-grekas
left a comment
There was a problem hiding this comment.
PR ready, I made some minor perf optims/cleanups FYI, nothing substantial.
|
Hi, I'm currently working on an OPcache-integrated user/static cache implementation, and I'm planning to submit an RFC and implementation soon. The implementation is not FrankenPHP-specific: it works under both NTS and ZTS, including FrankenPHP. Instead of using per-thread storage, it uses OPcache-managed shared-memory backends, so cached data can be shared across threads/processes while keeping request isolation. That is the main reason I think it may also fit the requirements of this PR. At a high level, the implementation adds separate OPcache-managed shared-memory backends for:
It also adds explicit cache APIs and static-cache attributes, including:
There are additional helper APIs in the current implementation as well, such as exists/delete/bulk-store/atomic-increment/status APIs, but the above are the main pieces relevant to this discussion. The storage path uses zero-copy/shared-memory representations where possible. Scalars are stored directly, eligible arrays and supported object graphs can use a shared-graph representation, and values that cannot use that path fall back to OPcache's binary serializer and, where needed, PHP serialization. User-defined objects and a vetted set of internal objects are supported, while resources and closures are rejected. One important semantic detail: The difference between the two attribute modes is:
The benchmark results below are from a prime-then-read HTTP workload (steady-state): each measured row is warmed up first, then measured with 5000 cache-hit operations per request. The measured requests had a 100% cache-hit ratio, with no build/store work included in the timing samples. I will publish the benchmark workload and raw payload-level results together with the RFC/PR. Current benchmark results on FrankenPHP: For reference, NTS php-fpm + nginx: In aggregate, this implementation is significantly faster than APCu in this read-heavy workload, not only on ZTS/FrankenPHP but also on a traditional NTS php-fpm + nginx setup. There are still workload-specific trade-offs, especially for small objects and serializer-fallback cases, so I intend to publish the raw per-payload benchmark data as part of the RFC. If there is interest in this direction, please let me know. I'll prioritize getting the RFC text, implementation PR, and benchmark suite ready for review. |
|
Thanks that's interesting. Not for this PR at this stage since we're looking for something that fits a simpler "message" style behavior, and also something that can be used on older versions of PHP. |
Second step of the split suggested in #2287: land the persistent-zval
subsystem as a standalone, reviewable header, independent of background
workers. This is the subsystem most likely to hide latent refcount or
memory-lifetime bugs; reviewing it in isolation is higher-signal than
finding issues inside a 3k-line diff.
What
persistent_zval.h(renamed from thebg_worker_vars.hdraft,prefix dropped for generality):
persistent_zval_validate— whitelist (scalars, arrays of allowedvalues, enum instances). Everything else fails fast.
persistent_zval_persist— deep-copy request → persistent (pemalloc)memory. Fast paths baked in: interned strings shared, opcache-
immutable arrays passed by pointer without copying or owning.
persistent_zval_free— deep-free; skips interned strings andimmutable arrays (borrowed, not owned).
persistent_zval_to_request— deep-copy persistent → fresh requestmemory. Enums re-resolved by class + case name on each read.
frankenphp.c: header included only whenFRANKENPHP_TEST_HOOKSisdefined. First real consumer (background workers) drops the guard.
Test hook gated on
FRANKENPHP_TEST_HOOKS:frankenphp_test_persist_roundtrip(mixed): mixedrunsvalidate → persist → to_request → free and returns the result.
zend_register_functionsat MINIT so it neverappears in
ext_functions[]and never ships in production builds.CI workflows set
-DFRANKENPHP_TEST_HOOKSinCGO_CFLAGS(
tests.yaml+sanitizers.yaml).windows.yamlis the releasebuild, not a test runner, and stays untouched.
Notes
unused-function warnings) and with it (test path).
FRANKENPHP_TEST_HOOKSguard around the header include goesaway in the PR that lands the first real caller; the test hook
itself goes away in that same step once end-to-end tests cover the
code paths.